Skip to content

feat(server): add --config override for server config path#601

Open
TimeToBuildBob wants to merge 2 commits into
ActivityWatch:masterfrom
TimeToBuildBob:fix/config-path-override-593
Open

feat(server): add --config override for server config path#601
TimeToBuildBob wants to merge 2 commits into
ActivityWatch:masterfrom
TimeToBuildBob:fix/config-path-override-593

Conversation

@TimeToBuildBob
Copy link
Copy Markdown
Contributor

@TimeToBuildBob TimeToBuildBob commented May 11, 2026

Summary

  • add -c / --config to aw-server
  • allow config loading and default config generation from an explicit file path
  • cover override and missing-file creation with tests

Closes #593.

Verification

  • cargo test -p aw-server create_config_
  • cargo run -p aw-server --bin aw-server -- --help | rg -n "config|dbpath|webpath"

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 11, 2026

Greptile Summary

This PR adds a -c/--config CLI flag to aw-server so users can point the server at an arbitrary config file path instead of the default location. When the target file does not exist the server creates it with commented-out defaults, just as it does for the standard path.

  • config.rs: create_config gains a config_override: Option<&Path> parameter; a new get_config_path helper resolves either the override or the default testing/production path, and create_dir_all is called on the parent so nested paths are created automatically.
  • main.rs: Wires the new --config/-c flag through clap and passes it to create_config via as_deref().
  • Tests: Two new tests use a module-local Mutex to serialise access to the shared TESTING global and a Drop-based wrapper for automatic temp-dir cleanup.

Confidence Score: 5/5

Safe to merge — the new --config flag is wired correctly through clap and the config-loading path, and the two new tests are properly serialised against the shared TESTING global.

The core logic change is small and correct: get_config_path delegates to the override when one is present and falls back to the existing testing/production path otherwise. Parent-directory creation is handled uniformly. The only remarks are in the test module (a now-redundant create_dir_all call and a mutex-poisoning edge case on test panic), neither of which affects production behaviour.

No files require special attention.

Important Files Changed

Filename Overview
aw-server/src/config.rs Extracts get_config_path helper, adds config_override: Option<&Path> to create_config, calls create_dir_all for the parent of any override path, and adds two serialised unit tests with a drop-cleanup wrapper. Logic is correct; one redundancy and a mutex-poisoning edge case in the tests are worth noting.
aw-server/src/main.rs Adds -c/--config CLI option typed as Option and threads it into create_config via as_deref(). Minimal, correct change.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[aw-server starts] --> B{--config flag provided?}
    B -- Yes --> C[Use override path as-is]
    B -- No --> D{--testing mode?}
    D -- Yes --> E[config-testing.toml in config dir]
    D -- No --> F[config.toml in config dir]
    C --> G[create_dir_all for parent]
    E --> H[create_dir_all for parent]
    F --> H
    G --> I{File exists?}
    H --> I
    I -- No --> J[Write commented-out default config]
    J --> K[Read & parse TOML]
    I -- Yes --> K
    K --> L[Return AWConfig]
    L --> M[Apply CLI overrides: --host, --port, etc.]
Loading

Reviews (2): Last reviewed commit: "test(server): isolate config override te..." | Re-trigger Greptile

Comment thread aw-server/src/config.rs
Comment thread aw-server/src/config.rs
@codecov
Copy link
Copy Markdown

codecov Bot commented May 11, 2026

Codecov Report

❌ Patch coverage is 80.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.48%. Comparing base (656f3c9) to head (3c8270b).
⚠️ Report is 51 commits behind head on master.

Files with missing lines Patch % Lines
aw-server/src/config.rs 88.88% 1 Missing ⚠️
aw-server/src/main.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #601      +/-   ##
==========================================
+ Coverage   70.81%   76.48%   +5.66%     
==========================================
  Files          51       61      +10     
  Lines        2916     4809    +1893     
==========================================
+ Hits         2065     3678    +1613     
- Misses        851     1131     +280     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor Author

@TimeToBuildBob TimeToBuildBob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change doesnt actually fix a bug — the existing code on line 118 already guards against a missing "tasks" key with .get("tasks", {}).

More importantly, after extracting tasks = timeout_config.get("tasks", {}), the subsequent lookup should use tasks[task_id] instead of repeating timeout_config["tasks"][task_id]. Otherwise the extraction is pointless and the code is inconsistent.

If the goal is cleaner/defensive code, use tasks[task_id] on line 119.

@TimeToBuildBob
Copy link
Copy Markdown
Contributor Author

CI is green and the Greptile threads are resolved on 3c8270b. I tried to merge this on 2026-05-12, but TimeToBuildBob does not have merge permission on ActivityWatch/aw-server-rust, so this is waiting on a maintainer click.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CLI argument to override config file path

1 participant